Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Editable installs for uv tool #5454

Merged
merged 4 commits into from
Jul 26, 2024
Merged

Editable installs for uv tool #5454

merged 4 commits into from
Jul 26, 2024

Conversation

blueraft
Copy link
Contributor

@blueraft blueraft commented Jul 25, 2024 •
edited
Loading

Summary

Resolves #5436.

Test Plan

cargo test

❯ ./target/debug/uv tool install -e ~/black
warning: `uv tool install` is experimental and may change without warning
Resolved 6 packages in 894ms
   Built black @ file:///Users/ahmedilyas/black
Prepared 1 package in 468ms
Installed 6 packages in 6ms
 + black==24.4.3.dev23+g7e2afc9 (from file:///Users/ahmedilyas/black)
 + click==8.1.7
 + mypy-extensions==1.0.0
 + packaging==24.1
 + pathspec==0.12.1
 + platformdirs==4.2.2
Installed 2 executables: black, blackd

venv has the .pth files.

❯ eza /Users/ahmedilyas/Library/Application\ Support/uv/tools/black/lib/python3.12/site-packages/
_black.pth       _virtualenv.py                         click                  mypy_extensions-1.0.0.dist-info  packaging                 pathspec                   platformdirs
_virtualenv.pth  black-24.4.3.dev23+g7e2afc9.dist-info  click-8.1.7.dist-info  mypy_extensions.py               packaging-24.1.dist-info  pathspec-0.12.1.dist-info  platformdirs-4.2.2.dist-info

charliermarsh
charliermarsh reviewed Jul 25, 2024
View reviewed changes
crates/uv/tests/tool_install.rs
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
"###);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we show the receipt here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a test for uv tool install back after uv tool install -e ./scripts/packages/black_editable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we show the receipt here?

uv tool install exits with a failure for the black_editable package since it has no executables. So no receipt gets created in this scenario.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an executable to it?

Copy link
Contributor Author

@blueraft blueraft Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a simple executable and added additional snapshot tests.

Can we also add a test for uv tool install back after uv tool install -e ./scripts/packages/black_editable?

I added this too, but it just installs black version from the PyPi server in this case, do you think that's the right behaviour here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's because you're creating a new context here. If you remove that, it retains the install (but the behavior is kind of confusing -- #5489 helps).

@charliermarsh
Copy link
Member

I'd like to merge #5489 before this.

@charliermarsh
Copy link
Member

Actually, it's ok to merge as-is. There are some confusing behaviors, but they also exist on main.

@charliermarsh charliermarsh enabled auto-merge (squash) July 26, 2024 19:56
@charliermarsh charliermarsh added enhancement New feature or improvement to existing functionality preview Experimental behavior labels Jul 26, 2024
@charliermarsh charliermarsh merged commit e8d7c0c into astral-sh:main Jul 26, 2024
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charliermarsh charliermarsh charliermarsh left review comments

Assignees
No one assigned
Labels
enhancement New feature or improvement to existing functionality preview Experimental behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Editable install of tools
2 participants
@blueraft @charliermarsh